Feat: check if system has creds saved, add new messages, extend backendSystem #3720

mikicvi-SAP · 2025-10-10T08:29:17Z

#3718

Check if the system has credentials stored; if not, don't save
- Log in end phase of the Fiori generator if not saving
Show the message attached to the system selection if the system does not have credentials stored
Show a warning message attached to the password input about the OS's password storage policy
If the username is saved with a system with incorrect credentials, pre-fill the username for reauth

…ndSystem

changeset-bot · 2025-10-10T08:29:21Z

🦋 Changeset detected

Latest commit: 7960995

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@sap-ux/odata-service-inquirer	Minor
@sap-ux/fiori-app-sub-generator	Patch
@sap-ux/deploy-config-sub-generator	Patch
@sap-ux/repo-app-import-sub-generator	Patch
@sap-ux/ui-service-inquirer	Patch
@sap-ux/ui-service-sub-generator	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

lfindlaysap

@mikicvi-SAP, here are my suggestions.

packages/fiori-app-sub-generator/src/translations/fioriAppSubGenerator.i18n.json

packages/odata-service-inquirer/src/translations/odata-service-inquirer.i18n.json

IainSAP · 2025-10-13T07:11:26Z

packages/fiori-app-sub-generator/src/fiori-app-generator/end.ts

        // No need to await, we cannot recover anyway
        // eslint-disable-next-line @typescript-eslint/no-floating-promises
        storeService.write(service.backendSystem);
+    } else if (service.backendSystem?.newOrUpdated === false) {


I think this condition should be inside (service.backendSystem && hostEnv !== hostEnvironment.bas) otherwise it will always log, even on bas.

Makes sense - thank you. Committed, with an additional check, as discussed.

…ithout-credentials

…s, enhance handling for temp creds, pre-fill username

…rwriting-saved-system-without-credentials

IainSAP · 2025-10-21T14:30:24Z

packages/fiori-app-sub-generator/src/fiori-app-generator/end.ts

+        service.backendSystem &&
+        hostEnv !== hostEnvironment.bas &&
+        service.backendSystem.newOrUpdated &&
+        !service.backendSystem.temporaryCredentials


Can we not remove the credentials from the PromptState.odataService.backendSystem object before returning (wherever we determine this) - so we dont need this extra logic in multiple places? I dont recall why we need this extra property temporaryCredentials?

I think we could do it with just newOrUpdated, actually, without the temporaryCredentials.

IainSAP · 2025-10-21T14:33:52Z

...odata-service-inquirer/src/prompts/datasources/sap-system/system-selection/prompt-helpers.ts

                );
+                (backendSystem as BackendSystemSelection).hasStoredCredentials = true;
+                return true;
+            } else if (errorType === ERROR_TYPE.AUTH && !hasStoredCredentials) {


Do we need this condition? Regardless of having creds or auth fails and the user will have to input credentials.

Not needed - leftover from testing something during dev, will remove it - thanks for spotting!

IainSAP · 2025-10-21T14:35:04Z

...odata-service-inquirer/src/prompts/datasources/sap-system/system-selection/prompt-helpers.ts

+        systemChoices = await Promise.all(
+            backendSystems.map(async (system) => {
+                // Check if the system has stored credentials by reading it with sensitive data
+                const systemWithCredentials = await new SystemService(LoggerHelper.logger).read(


This is reading every system from secure store? We went to some effort to avoid this!

…rompts, tests

…rwriting-saved-system-without-credentials

sonarqubecloud · 2025-10-23T09:06:14Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
93.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

IainSAP · 2025-10-23T09:41:36Z

packages/odata-service-inquirer/src/prompts/datasources/sap-system/credentials/questions.ts

+                const selectedSystem = answers?.[promptNames.systemSelection] as SystemSelectionAnswerType;
+                if (selectedSystem?.type === 'backendSystem') {
+                    const selectedBackendSystem = selectedSystem.system as BackendSystem;
+                    if (selectedBackendSystem?.userDisplayName) {


Why check for display name here? I dont think this is related to the username.

IainSAP · 2025-10-23T09:42:12Z

packages/odata-service-inquirer/src/prompts/datasources/sap-system/credentials/questions.ts

                return valResult;
            },
            additionalMessages: (password: string, answers: T) => {
+                if (PromptState.odataService.connectedSystem?.backendSystem?.newOrUpdated) {


This should be a lower priority that the cert errors (the condition should be evaluated after error conditions)

IainSAP · 2025-10-23T09:47:39Z

...odata-service-inquirer/src/prompts/datasources/sap-system/system-selection/prompt-helpers.ts

            );
        } else if (backendSystem.authenticationType === 'basic' || !backendSystem.authenticationType) {
+            const hasStoredCredentials = !!(backendSystem.username && backendSystem.password);
+            PromptState.hasStoredCredentials = hasStoredCredentials;


Im not sure we should be maintaining a specific system info at the root level without context. Can we instead update the locally cached backend system (selected system actually) that is selected with updateCredentials boolean property that is set based on some criteria (existence of credentials or a property of the stored system in future marked as do not store creds).

IainSAP · 2025-10-23T11:09:59Z

packages/odata-service-inquirer/src/prompts/datasources/sap-system/credentials/questions.ts

+                    const selectedBackendSystem = selectedSystem.system as BackendSystem;
+                    if (selectedBackendSystem?.userDisplayName) {
+                        // Read system with credentials to get the stored username, since we won't assume that displayName = username
+                        const systemWithCredentials = await new SystemService(LoggerHelper.logger).read(


I think we can avoid a second read here.

IainSAP · 2025-10-23T11:12:49Z

...odata-service-inquirer/src/prompts/datasources/sap-system/system-selection/prompt-helpers.ts

-                typeof backendSystem.username === 'string' &&
-                typeof backendSystem.password === 'string'
-            ) {
+            if (errorType === ERROR_TYPE.AUTH) {


We shouldnt be validating if there are no credentials to validate hence the conditions that have been removed? Note that this code is also used by new system flows probably and so these conditions are relevant in those cases

This was meant to check with hasStoredCredentials from above, which would be valid at that point. I missed it.

feat: check if system has creds saved, add new messages, extend backe…

6fa20c7

…ndSystem

mikicvi-SAP self-assigned this Oct 10, 2025

mikicvi-SAP added 2 commits October 10, 2025 09:59

feat: fix translation path for info message

7139bf6

feat: remove temporaryCredentials handling from backend system logic

41d4f6f

mikicvi-SAP marked this pull request as ready for review October 10, 2025 10:45

mikicvi-SAP requested a review from a team as a code owner October 10, 2025 10:45

feat: add warning message to the password prompt

4764c2f

lfindlaysap requested changes Oct 10, 2025

View reviewed changes

IainSAP reviewed Oct 13, 2025

View reviewed changes

mikicvi-SAP marked this pull request as draft October 17, 2025 09:22

mikicvi-SAP and others added 6 commits October 17, 2025 10:23

Merge branch 'main' into feat/3718/prevent-overwriting-saved-system-w…

1770221

…ithout-credentials

feat: address translations feedback

69d3306

feat: extend local type, load systems from store in helpers with cred…

c32a0a9

…s, enhance handling for temp creds, pre-fill username

Merge remote-tracking branch 'origin/main' into feat/3718/prevent-ove…

6ed8b5d

…rwriting-saved-system-without-credentials

Linting auto fix commit

45e7ed5

feat: unit tests for questions

d7e7e17

IainSAP reviewed Oct 21, 2025

View reviewed changes

mikicvi-SAP added 4 commits October 23, 2025 09:25

feat: refactor backend system handling and credential management in p…

6c97706

…rompts, tests

Merge remote-tracking branch 'origin/main' into feat/3718/prevent-ove…

fbe4872

…rwriting-saved-system-without-credentials

feat: changeset, remove unnecessary test

8a6a04a

feat: fix code smells

de02eb7

Linting auto fix commit

7960995

mikicvi-SAP marked this pull request as ready for review October 23, 2025 09:08

IainSAP reviewed Oct 23, 2025

View reviewed changes

Feat: check if system has creds saved, add new messages, extend backendSystem #3720

Are you sure you want to change the base?

Feat: check if system has creds saved, add new messages, extend backendSystem #3720

Uh oh!

Conversation

mikicvi-SAP commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

lfindlaysap left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IainSAP Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IainSAP Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 23, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IainSAP Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IainSAP Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IainSAP Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mikicvi-SAP commented Oct 10, 2025 •

edited

Loading

changeset-bot bot commented Oct 10, 2025 •

edited

Loading

IainSAP Oct 21, 2025 •

edited

Loading

IainSAP Oct 21, 2025 •

edited

Loading

IainSAP Oct 23, 2025 •

edited

Loading

IainSAP Oct 23, 2025 •

edited

Loading

IainSAP Oct 23, 2025 •

edited

Loading